gh-116631: Fix race condition in test_shutdown_immediate_put_join#116670
gh-116631: Fix race condition in test_shutdown_immediate_put_join#116670colesbury merged 2 commits intopython:mainfrom
test_shutdown_immediate_put_join#116670Conversation
…oin` The test case had a race condition: if `q.task_done()` was executed after `shutdown(immediate=True)`, then it would raise an exception because the immediate shutdown already emptied the queue. This happened rarely with the GIL (due to the switching interval), but frequently in the free-threaded build.
|
@EpicWink - I wasn't able to add you as a reviewer, but if you have time, I'd appreciate your feedback on the PR as well. |
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
I don't understand this test well enough to approve the change -- we ought to wait for @EpicWink, since it's his code, but sometimes it takes a week to get his input (he's just a volunteer). If this race condition is causing you grief, go ahead and merge it. I have one naming nit.
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
|
@YvesDup wrote the tests, with minimal fixing by me before the PR was merged. I've found adding delays in the main thread usually surfaces the race conditions. I'll test when I'm on PC |
EpicWink
left a comment
EpicWink
left a comment
There was a problem hiding this comment.
Tests succeed even with random delays spotted around
| result = q.get() | ||
| self.assertEqual(result, "Y") | ||
| q.task_done() | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Thanks @EpicWink for the review. @colesbury Go ahead and merge! (No need to credit me for that variable rename. :-)
|
@EpicWink, thanks for reviewing and testing this! |
…oin` (python#116670) The test case had a race condition: if `q.task_done()` was executed after `shutdown(immediate=True)`, then it would raise an exception because the immediate shutdown already emptied the queue. This happened rarely with the GIL (due to the switching interval), but frequently in the free-threaded build.
…oin` (python#116670) The test case had a race condition: if `q.task_done()` was executed after `shutdown(immediate=True)`, then it would raise an exception because the immediate shutdown already emptied the queue. This happened rarely with the GIL (due to the switching interval), but frequently in the free-threaded build.
…oin` (python#116670) The test case had a race condition: if `q.task_done()` was executed after `shutdown(immediate=True)`, then it would raise an exception because the immediate shutdown already emptied the queue. This happened rarely with the GIL (due to the switching interval), but frequently in the free-threaded build.
3 participants
The test case had a race condition: if
q.task_done()was executed aftershutdown(immediate=True), then it would raise an exception because the shutdown call already emptied the queue. This happened rarely with the GIL (due to the switching interval), but frequently in the free-threaded build with the GIL disabled.test_queue.test_shutdown_immediate_put_join#116631